Repository navigation
Conversation
The event_time filter that dbt applies to upstream refs of a microbatch model compared the column with a 'YYYY-MM-DD hh:mm:ss' string literal. ClickHouse casts the literal to the column type, which fails for Date and Date32 columns (TYPE_MISMATCH). Compare with toDateTime(...) instead, which works for Date, Date32, DateTime and DateTime64 and matches the predicates the microbatch strategy already uses on the target table. Fixes ClickHouse#527
76fb61b to
a47e60a
Compare
|
@koletzilla could you take a look at this when you get a chance? It fixes #527, where a microbatch model with a |
There was a problem hiding this comment.
🟡 Changes recommended
The new toDateTime boundaries exclude valid historical dates and can shift sampling windows when server and column timezones differ, requiring corrections and regression coverage.
2 open issues
What changed in this PR
This PR addresses #527 by changing upstream event-time filters to use typed datetime boundaries, allowing microbatch models with Date event-time columns to run without string-conversion errors. The same rendering change applies to --sample.
Changes:
- Add a helper that renders filter boundaries with
toDateTime(). - Add unit coverage for boundary combinations and an integration scenario using a
Datecolumn. - Document the fix in the changelog.
| File | Description |
|---|---|
tests/unit/test_relation.py |
Tests paired, start-only, end-only, and empty filter bounds. |
tests/integration/adapter/incremental/test_incremental_microbatch.py |
Adds a microbatch scenario with a Date event-time column. |
dbt/adapters/clickhouse/relation.py |
Replaces bare string boundaries with typed datetime expressions. |
CHANGELOG.md |
Records the date-column filtering fix. |
Review effort: Balanced
| def _to_datetime_literal(value: datetime) -> str: | ||
| # A bare string literal is cast to the column type, and '2020-01-01 00:00:00' cannot be cast to Date | ||
| # or Date32. A DateTime value compares with Date, Date32, DateTime and DateTime64 columns alike. | ||
| return f"toDateTime('{value.strftime('%Y-%m-%d %H:%M:%S')}')" |
There was a problem hiding this comment.
The delete predicates in incremental.sql have rendered the boundaries as toDateTime('...') since the microbatch strategy was added, so they are in the session timezone, and the upstream filter now uses the same form. On 26.9 with session_timezone='Europe/Paris', toDateTime64('...', 0) resolves to the same instant as toDateTime('...') for DateTime, DateTime('UTC') and DateTime64(3, 'UTC') columns. Before this PR the filter on a DateTime('UTC') column used the column zone while the delete used the server zone, so on a non-UTC server a re-run of a batch did not delete the rows it had inserted in the last hour of the window; one form on both sides fixes that. Forcing 'UTC' would change the result for plain DateTime columns on non-UTC servers, which is a separate decision from #527, so the zone stays as it was.
| # or Date32. A DateTime value compares with Date, Date32, DateTime and DateTime64 columns alike. | ||
| return f"toDateTime('{value.strftime('%Y-%m-%d %H:%M:%S')}')" |
There was a problem hiding this comment.
Agreed: toDateTime('1960-06-15 00:00:00') clamps to 1970-01-01 00:00:00, so a Date32 or DateTime64 window before 1970 selected nothing. Both the filter (relation.py) and the delete predicates (incremental.sql) now render toDateTime64('...', 0), which compares correctly with Date, Date32, DateTime and DateTime64 columns on 26.9 and keeps the 1960 bounds. The unit test covers 1960 and 2200 boundaries, and two new integration scenarios run the microbatch flow with begin=1960-01-01 on Date32 and DateTime64 columns; both produced an empty model before the change.
toDateTime() clamps every value to 1970-2106, so a Date32 or DateTime64
event_time column with rows before 1970 got an empty batch window: the
filter on upstream models selected nothing and the delete predicates on
the model matched nothing. Render both as toDateTime64('...', 0), which
compares with Date, Date32, DateTime and DateTime64 columns alike and
covers 1900-2299.
The delete predicates in incremental.sql are changed in the same way so
the rows a batch deletes are the rows it selects.
Adds the pre-1970 and post-2106 cases to the unit test and two microbatch
integration scenarios with a 1960 begin on Date32 and DateTime64 columns.
|
@BentsiLeviav since your review request I switched the boundary literal to |

Summary
Fixes #527.
A microbatch model whose
event_timeis aDate(orDate32) column fails on its first batch:The error comes from the filter dbt adds to upstream
ref()s of a microbatch model, rendered byClickHouseRelation._render_event_time_filtered. It compared the column with a'YYYY-MM-DD hh:mm:ss'string literal. ClickHouse casts that literal to the column type, and a value with a time part cannot be cast toDate.The filter now compares the column with
toDateTime('YYYY-MM-DD hh:mm:ss'). That works againstDate,Date32,DateTimeandDateTime64columns. It is also the form the microbatch strategy already uses for its delete predicates on the target table (incremental.sql), so both sides of a batch now use the same boundaries. ForDateTimecolumns without an explicit time zone the result does not change. The same filter is used by--samplemode.Tests:
tests/unit/test_relation.py::test_render_event_time_filteredcovers the start+end, start-only, end-only and empty cases.TestMicrobatchIncrementalDateEventTimeruns the standardBaseMicrobatchscenario on aDateevent_time column. It fails onmainwith the error above and passes with this change.Checklist
Note
Medium Risk
Changes SQL predicates for microbatch incremental and event-time sampling; incorrect boundaries could skip or mis-delete rows, though behavior is narrowly scoped and heavily tested.
Overview
Fixes microbatch incremental runs (and upstream/
--sampleevent-time filters) that failed whenevent_timeisDateorDate32, because batch boundaries were compared using'YYYY-MM-DD hh:mm:ss'string literals that ClickHouse cannot cast toDate.ClickHouseRelation._render_event_time_filterednow builds predicates with a sharedtoDateTime64(..., 0)helper instead of quoted datetime strings. The microbatch delete/insert predicates inincremental.sqlare aligned to the sametoDateTime64form so upstream filtering and target deletes share identical batch windows.Using
DateTime64also avoidsDateTime’s 1970–2106 clamping, so batches before 1970 or after 2106 work forDate32/DateTime64columns. Coverage adds unit tests for filter rendering and integration tests forDate,Date32, and pre-1970DateTime64microbatch scenarios.Reviewed by Cursor Bugbot for commit f45a498. Bugbot is set up for automated code reviews on this repo. Configure here.